Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix flaky integration test: TestDatabaseAccessMongoConnectionCount #10869

Merged
merged 2 commits into from
Mar 8, 2022

Conversation

greedy52
Copy link
Contributor

@greedy52 greedy52 commented Mar 4, 2022

Seeing this fail in CI build and it's quite easy to reproduce locally.

So problem is, per client, it may establish 6 or 9 connections. Server connection count is NOT growing one client after another, BUT getting 6 != 9 sometimes. <--- fix to use require.InDelta

Another potential issue is we are getting server count after client disconnect. There is an race condition that server may have 0 connections or it could still have all 9 connections. <--- fix to get the server connection count before disconnect

Lastly, added one additional check to wait until the server reaches 0 connection count in the end.

Updates #9492

@greedy52 greedy52 self-assigned this Mar 4, 2022
@greedy52 greedy52 requested a review from smallinsky March 4, 2022 20:46
@github-actions github-actions bot requested review from atburke and nklaassen March 4, 2022 20:46
Copy link
Contributor

@smallinsky smallinsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks for the fix.

@jakule
Copy link
Contributor

jakule commented Mar 7, 2022

@greedy52 From what I understand the problem comes from the fact that mongo driver creates multiple TCP connections per one DB connection (if you can phrase it in this way). Have you tried to use some driver options to control that?
For example SetMaxPoolSize() https://github.com/mongodb/mongo-go-driver/blob/ae48c67470353306985af3466d20d1241d972296/mongo/options/clientoptions.go#L575-L581 ?
I think that assumption that the connection count is +/- 3 can bite us when if the driver changes the behaviour (after update for example).

@greedy52
Copy link
Contributor Author

greedy52 commented Mar 7, 2022

@greedy52 From what I understand the problem comes from the fact that mongo driver creates multiple TCP connections per one DB connection (if you can phrase it in this way). Have you tried to use some driver options to control that? For example SetMaxPoolSize() https://github.com/mongodb/mongo-go-driver/blob/ae48c67470353306985af3466d20d1241d972296/mongo/options/clientoptions.go#L575-L581 ? I think that assumption that the connection count is +/- 3 can bite us when if the driver changes the behaviour (after update for example).

@jakule I have tried SetMaxPoolSize and it has no effect. So the same single connection is used for auth, ping, and our query. (so effectively max pool size is 1 without explicitly setting it). However, I am seeing other connections are spamming isMaster calls and they don't respect the pool size. In fact, if you set min pool size to a bigger number say 10, you may 30 connections. Something is fishy. Let me dig into mongo driver a little bit more.

Copy link
Contributor

@jakule jakule left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@greedy52
Copy link
Contributor Author

greedy52 commented Mar 7, 2022

@jakule @smallinsky as suggested by Jakub, I did some more digging into this and got some bad news.

The mongo client does have a connection pool, which controls connection pool size and can be monitored by options.Client().SetPoolMonitor. In our tests, there is only one connection created in the pool.

So besides the connection pool, the client also creates TWO other connections to the server that is not from this pool. For example, a background rttMonitor creates a separate connection to send isMaster.

So, our mongo engine receives 3 connections per client. The worst part is we are multiplying this since we use the same driver. For each incoming connection to the engine, we establish 3 to the actual server. This multiplies. So 3 * 3 = 9.

I guess one of the background tasks doesn't get caught all the time, so we are seeing 2 * 3 = 6 from time to time.

I don't the multiplying effect will be an easy fix. I will create an separate issue to track

@greedy52 greedy52 enabled auto-merge (squash) March 8, 2022 13:12
@greedy52 greedy52 merged commit 93a89a0 into master Mar 8, 2022
@greedy52 greedy52 deleted the STeve/fix_flaky_mongo_integration_test branch March 8, 2022 13:35
@zmb3
Copy link
Collaborator

zmb3 commented Mar 8, 2022

@greedy52 can we get this backported to v9 and v8?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants